Skip to content

fix: presence stream stalling intermittently (#20090) - #282

Merged
FrenchGithubUser merged 1 commit into
masterfrom
tt/fix-presence-stream
Aug 14, 2026
Merged

fix: presence stream stalling intermittently (#20090)#282
FrenchGithubUser merged 1 commit into
masterfrom
tt/fix-presence-stream

Conversation

@FrenchGithubUser

@FrenchGithubUser FrenchGithubUser commented Aug 14, 2026

Copy link
Copy Markdown
Member

cherry picked the resulting squash commit of element-hq/synapse#20090

@FrenchGithubUser
fix: presence stream stalling intermittently (#20090)
6733bf6 
This is a fix for presence updates silently stalling when a `/sync`
request is cancelled mid-write, causing a stream ID to be leaked into
`_unfinished_ids` and permanently pinning the persisted stream position.

Fixes element-hq/synapse#19800

### Pull Request Checklist

<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->

* [x] Pull request is based on the develop branch
* [x] Pull request includes a [changelog
file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog).
The entry should:
- Be a short description of your change which makes sense to users.
"Fixed a bug that prevented receiving messages from other servers."
instead of "Moved X method from `EventStore` to `EventWorkerStore`.".
  - Use markdown where necessary, mostly for `code blocks`.
  - End with either a period (.) or an exclamation mark (!).
  - Start with a capital letter.
- Feel free to credit yourself, by adding a sentence "Contributed by
@github_username." or "Contributed by [Your Name]." to the end of the
entry.
* [x] [Code
style](https://element-hq.github.io/synapse/latest/code_style.html) is
correct (run the
[linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
@FrenchGithubUser
FrenchGithubUser marked this pull request as ready for review August 14, 2026 09:27
@FrenchGithubUser
FrenchGithubUser requested a review from a team as a code owner August 14, 2026 09:27
@cursor

cursor Bot commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Medium Risk
Touches core stream ID allocation used by presence and other streams; incorrect cleanup could advance tokens incorrectly, but the change mirrors existing failure-path behavior and is covered by a targeted regression test.

Overview
Fixes presence (and other multi-writer streams) stalling when a cancellable /sync is aborted while a write is still allocating a stream ID via MultiWriterIdGenerator.get_next().

MultiWriterIdGenerator (_MultiWriterCtxManager) now records allocated IDs inside the DB callback before __aenter__ returns, and on BaseException (including CancelledError / GeneratorExit) marks those IDs finished and notifies replication—the same cleanup __aexit__ would do on failure, which never runs if __aenter__ never completes.

Adds test_cancelled_enter_does_not_wedge_position to reproduce the wedged token after a cancelled allocation, plus a changelog entry for #20090.

Reviewed by Cursor Bugbot for commit 6733bf6. Bugbot is set up for automated code reviews on this repo. Configure here.

@codecov

codecov Bot commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.73%. Comparing base (013821e) to head (6733bf6).

Files with missing lines Patch % Lines
synapse/storage/util/id_generators.py 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #282      +/-   ##
==========================================
- Coverage   80.73%   80.73%   -0.01%     
==========================================
  Files         503      503              
  Lines       72286    72296      +10     
  Branches    10872    10873       +1     
==========================================
+ Hits        58363    58371       +8     
- Misses      10658    10659       +1     
- Partials     3265     3266       +1
Files with missing lines Coverage Δ
synapse/storage/util/id_generators.py 90.72% <90.90%> (-0.03%) ⬇️

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 013821e...6733bf6. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@FrenchGithubUser
FrenchGithubUser added this pull request to the merge queue Aug 14, 2026
Merged via the queue into master with commit 6cef6ed Aug 14, 2026
53 of 56 checks passed
@FrenchGithubUser
FrenchGithubUser deleted the tt/fix-presence-stream branch August 14, 2026 12:07
@jason-famedly jason-famedly mentioned this pull request Aug 20, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nico-famedly nico-famedly nico-famedly approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@FrenchGithubUser @nico-famedly